fix(a11y): Accessibility improvements for WCAG 2.1 Level A compliance#38342
Closed
Aitema-gmbh wants to merge 44 commits intoapache:masterfrom
Closed
fix(a11y): Accessibility improvements for WCAG 2.1 Level A compliance#38342Aitema-gmbh wants to merge 44 commits intoapache:masterfrom
Aitema-gmbh wants to merge 44 commits intoapache:masterfrom
Conversation
added 3 commits
March 2, 2026 14:23
- SkipLink: WCAG 2.4.1 Bypass Blocks - skip-to-content navigation for keyboard users - StatusAnnouncer: WCAG 4.1.2 Name, Role, Value - ARIA live region for screen readers - Fixed tabindex handling: temporary tabindex on focus, removed on blur - Uses current @apache-superset/core/ui import structure
Wraps chart titles in semantic <h2> elements for screen reader accessibility. The heading inherits all visual styles from the parent container so the visual appearance remains unchanged.
- SkipLink: tests for rendering, focus management, temporary tabindex, existing tabindex preservation, and fallback hash navigation - StatusAnnouncer: tests for ARIA live regions, polite/assertive messages, message timeout clearing, and provider rendering
4 tasks
dosubot
Bot
added
change:frontend
Contributor
Author
|
Hi @geido @kgabryje @michael-s-molina @rusackas 👋 This is a clean rewrite of #37211, rebased on current
The scope is intentionally focused: 2 new accessibility components (SkipLink, StatusAnnouncer) + 1 small semantic heading change. Unit tests included. Would really appreciate a review when you have time! Happy to address any feedback. 🙏 |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Comment on lines
+100
to
+114
| it('prevents default link behavior', () => { | ||
| const targetEl = document.createElement('div'); | ||
| targetEl.id = 'main-content'; | ||
| document.body.appendChild(targetEl); | ||
|
|
||
| render(<SkipLink />); | ||
| const link = screen.getByText('Skip to main content'); | ||
|
|
||
| const preventDefaultSpy = jest.fn(); | ||
| link.addEventListener('click', preventDefaultSpy, { once: true }); | ||
|
|
||
| fireEvent.click(link); | ||
|
|
||
| document.body.removeChild(targetEl); | ||
| }); |
Contributor
There was a problem hiding this comment.
Suggestion: The "prevents default link behavior" test sets up a spy but never makes any assertion about it, so the test always passes and does not actually verify that the default navigation is prevented; add a concrete assertion that the location hash remains unchanged when the target exists to ensure the behavior is exercised. [logic error]
Severity Level: Major ⚠️
- ⚠️ SkipLink default-prevention regressions not detected by unit tests.
- ⚠️ Accessibility behavior may break without automated test signal.
Suggested change
| it('prevents default link behavior', () => { | |
| const targetEl = document.createElement('div'); | |
| targetEl.id = 'main-content'; | |
| document.body.appendChild(targetEl); | |
| render(<SkipLink />); | |
| const link = screen.getByText('Skip to main content'); | |
| const preventDefaultSpy = jest.fn(); | |
| link.addEventListener('click', preventDefaultSpy, { once: true }); | |
| fireEvent.click(link); | |
| document.body.removeChild(targetEl); | |
| }); | |
| it('prevents default link behavior', () => { | |
| const targetEl = document.createElement('div'); | |
| targetEl.id = 'main-content'; | |
| document.body.appendChild(targetEl); | |
| // Set an initial hash to detect unintended navigation | |
| window.location.hash = '#initial'; | |
| render(<SkipLink />); | |
| const link = screen.getByText('Skip to main content'); | |
| fireEvent.click(link); | |
| // Clicking the skip link should not change the location hash when the target exists | |
| expect(window.location.hash).toBe('#initial'); | |
| document.body.removeChild(targetEl); | |
| }); |
Steps of Reproduction ✅
1. Run the unit tests for the accessibility components with `npm test --
SkipLink.test.tsx`, which executes
`superset-frontend/src/components/Accessibility/SkipLink.test.tsx`.
2. Observe the test case `it('prevents default link behavior', () => { ... })` at lines
100–114 in `SkipLink.test.tsx`; it sets up a `preventDefaultSpy` and calls
`fireEvent.click(link)` but contains no `expect(...)` assertions.
3. In `superset-frontend/src/components/Accessibility/SkipLink.tsx` (the component
imported via `import SkipLink from './SkipLink';`), temporarily remove or comment out the
logic that prevents default navigation on click (e.g., the `event.preventDefault()` call
in the click handler).
4. Re-run `npm test -- SkipLink.test.tsx` and note that all tests, including `'prevents
default link behavior'`, still pass despite the regression, because the test at lines
100–114 never checks either the spy or `window.location.hash`, so it cannot fail based on
the default-prevention behavior.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/components/Accessibility/SkipLink.test.tsx
**Line:** 100:114
**Comment:**
*Logic Error: The "prevents default link behavior" test sets up a spy but never makes any assertion about it, so the test always passes and does not actually verify that the default navigation is prevented; add a concrete assertion that the location hash remains unchanged when the target exists to ensure the behavior is exercised.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Comment on lines
+25
to
+90
| useMemo, | ||
| useRef, | ||
| useState, | ||
| } from 'react'; | ||
| import { css } from '@apache-superset/core/ui'; | ||
|
|
||
| /** | ||
| * StatusAnnouncer - WCAG 4.1.2 Name, Role, Value | ||
| * Provides an ARIA live region for announcing dynamic content updates | ||
| * to assistive technologies (screen readers). | ||
| */ | ||
|
|
||
| type Politeness = 'polite' | 'assertive'; | ||
|
|
||
| interface StatusAnnouncerContextType { | ||
| announce: (message: string, politeness?: Politeness) => void; | ||
| } | ||
|
|
||
| const StatusAnnouncerContext = createContext<StatusAnnouncerContextType>({ | ||
| announce: () => {}, | ||
| }); | ||
|
|
||
| export const useStatusAnnouncer = () => useContext(StatusAnnouncerContext); | ||
|
|
||
| const visuallyHiddenStyle = css` | ||
| position: absolute; | ||
| width: 1px; | ||
| height: 1px; | ||
| padding: 0; | ||
| margin: -1px; | ||
| overflow: hidden; | ||
| clip: rect(0, 0, 0, 0); | ||
| white-space: nowrap; | ||
| border: 0; | ||
| `; | ||
|
|
||
| interface StatusAnnouncerProviderProps { | ||
| children: ReactNode; | ||
| } | ||
|
|
||
| export const StatusAnnouncerProvider: FC<StatusAnnouncerProviderProps> = ({ | ||
| children, | ||
| }) => { | ||
| const [politeMessage, setPoliteMessage] = useState(''); | ||
| const [assertiveMessage, setAssertiveMessage] = useState(''); | ||
| const clearTimeoutRef = useRef<ReturnType<typeof setTimeout>>(); | ||
|
|
||
| const announce = useCallback( | ||
| (message: string, politeness: Politeness = 'polite') => { | ||
| if (clearTimeoutRef.current) { | ||
| clearTimeout(clearTimeoutRef.current); | ||
| } | ||
|
|
||
| if (politeness === 'assertive') { | ||
| setAssertiveMessage(message); | ||
| } else { | ||
| setPoliteMessage(message); | ||
| } | ||
|
|
||
| clearTimeoutRef.current = setTimeout(() => { | ||
| setPoliteMessage(''); | ||
| setAssertiveMessage(''); | ||
| }, 5000); | ||
| }, | ||
| [], | ||
| ); |
Contributor
There was a problem hiding this comment.
Suggestion: The timeout that clears the polite/assertive messages is never cleaned up on unmount, so if the provider is unmounted within 5 seconds of an announcement the timeout callback will still fire and attempt to update state on an unmounted component, causing React warnings and a small memory/resource leak; add a cleanup that clears any pending timeout when the provider unmounts. [resource leak]
Severity Level: Major ⚠️
- ⚠️ React warns about state updates after unmount.
- ⚠️ Tests using StatusAnnouncerProvider may become flaky.
- ⚠️ Minor timer leak when unmounting within five seconds.
Suggested change
| useMemo, | |
| useRef, | |
| useState, | |
| } from 'react'; | |
| import { css } from '@apache-superset/core/ui'; | |
| /** | |
| * StatusAnnouncer - WCAG 4.1.2 Name, Role, Value | |
| * Provides an ARIA live region for announcing dynamic content updates | |
| * to assistive technologies (screen readers). | |
| */ | |
| type Politeness = 'polite' | 'assertive'; | |
| interface StatusAnnouncerContextType { | |
| announce: (message: string, politeness?: Politeness) => void; | |
| } | |
| const StatusAnnouncerContext = createContext<StatusAnnouncerContextType>({ | |
| announce: () => {}, | |
| }); | |
| export const useStatusAnnouncer = () => useContext(StatusAnnouncerContext); | |
| const visuallyHiddenStyle = css` | |
| position: absolute; | |
| width: 1px; | |
| height: 1px; | |
| padding: 0; | |
| margin: -1px; | |
| overflow: hidden; | |
| clip: rect(0, 0, 0, 0); | |
| white-space: nowrap; | |
| border: 0; | |
| `; | |
| interface StatusAnnouncerProviderProps { | |
| children: ReactNode; | |
| } | |
| export const StatusAnnouncerProvider: FC<StatusAnnouncerProviderProps> = ({ | |
| children, | |
| }) => { | |
| const [politeMessage, setPoliteMessage] = useState(''); | |
| const [assertiveMessage, setAssertiveMessage] = useState(''); | |
| const clearTimeoutRef = useRef<ReturnType<typeof setTimeout>>(); | |
| const announce = useCallback( | |
| (message: string, politeness: Politeness = 'polite') => { | |
| if (clearTimeoutRef.current) { | |
| clearTimeout(clearTimeoutRef.current); | |
| } | |
| if (politeness === 'assertive') { | |
| setAssertiveMessage(message); | |
| } else { | |
| setPoliteMessage(message); | |
| } | |
| clearTimeoutRef.current = setTimeout(() => { | |
| setPoliteMessage(''); | |
| setAssertiveMessage(''); | |
| }, 5000); | |
| }, | |
| [], | |
| ); | |
| useEffect, | |
| useMemo, | |
| useRef, | |
| useState, | |
| } from 'react'; | |
| import { css } from '@apache-superset/core/ui'; | |
| /** | |
| * StatusAnnouncer - WCAG 4.1.2 Name, Role, Value | |
| * Provides an ARIA live region for announcing dynamic content updates | |
| * to assistive technologies (screen readers). | |
| */ | |
| type Politeness = 'polite' | 'assertive'; | |
| interface StatusAnnouncerContextType { | |
| announce: (message: string, politeness?: Politeness) => void; | |
| } | |
| const StatusAnnouncerContext = createContext<StatusAnnouncerContextType>({ | |
| announce: () => {}, | |
| }); | |
| export const useStatusAnnouncer = () => useContext(StatusAnnouncerContext); | |
| const visuallyHiddenStyle = css` | |
| position: absolute; | |
| width: 1px; | |
| height: 1px; | |
| padding: 0; | |
| margin: -1px; | |
| overflow: hidden; | |
| clip: rect(0, 0, 0, 0); | |
| white-space: nowrap; | |
| border: 0; | |
| `; | |
| interface StatusAnnouncerProviderProps { | |
| children: ReactNode; | |
| } | |
| export const StatusAnnouncerProvider: FC<StatusAnnouncerProviderProps> = ({ | |
| children, | |
| }) => { | |
| const [politeMessage, setPoliteMessage] = useState(''); | |
| const [assertiveMessage, setAssertiveMessage] = useState(''); | |
| const clearTimeoutRef = useRef<ReturnType<typeof setTimeout>>(); | |
| const announce = useCallback( | |
| (message: string, politeness: Politeness = 'polite') => { | |
| if (clearTimeoutRef.current) { | |
| clearTimeout(clearTimeoutRef.current); | |
| } | |
| if (politeness === 'assertive') { | |
| setAssertiveMessage(message); | |
| } else { | |
| setPoliteMessage(message); | |
| } | |
| clearTimeoutRef.current = setTimeout(() => { | |
| setPoliteMessage(''); | |
| setAssertiveMessage(''); | |
| }, 5000); | |
| }, | |
| [], | |
| ); | |
| useEffect(() => { | |
| return () => { | |
| if (clearTimeoutRef.current) { | |
| clearTimeout(clearTimeoutRef.current); | |
| } | |
| }; | |
| }, []); |
Steps of Reproduction ✅
1. Render `StatusAnnouncerProvider` (default export from
`superset-frontend/src/components/Accessibility/StatusAnnouncer.tsx:65`) in a React tree
with any child component.
2. Inside a child component, call `const { announce } = useStatusAnnouncer();
announce('Loading...', 'polite');` which executes the callback in `announce()` at
`StatusAnnouncer.tsx:72-88`, setting `clearTimeoutRef.current = setTimeout(...)`.
3. Unmount the subtree containing `StatusAnnouncerProvider` within 5 seconds (for example,
in a route change or in a React Testing Library test calling `unmount()` on the rendered
tree).
4. After unmount, the pending timeout fires and runs the callback at
`StatusAnnouncer.tsx:84-86`, calling `setPoliteMessage('')` / `setAssertiveMessage('')` on
an unmounted component, which in React produces "Can't perform a React state update on an
unmounted component" warnings and keeps the timer alive until it finishes.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/components/Accessibility/StatusAnnouncer.tsx
**Line:** 25:90
**Comment:**
*Resource Leak: The timeout that clears the polite/assertive messages is never cleaned up on unmount, so if the provider is unmounted within 5 seconds of an announcement the timeout callback will still fire and attempt to update state on an unmounted component, causing React warnings and a small memory/resource leak; add a cleanup that clears any pending timeout when the provider unmounts.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Agent Run #e1a659
Actionable Suggestions - 1
-
superset-frontend/src/components/Accessibility/SkipLink.tsx - 1
- Missing translation for user-facing text · Line 63-63
Additional Suggestions - 1
-
superset-frontend/src/components/Accessibility/SkipLink.test.tsx - 1
-
Incomplete Test Assertion · Line 100-114The test 'prevents default link behavior' sets up a spy on the click event but lacks an assertion, so it doesn't verify the claimed behavior. Per BITO.md [6262], tests must assert actual logic beyond mere rendering. This could allow silent failures in event handling.
Code suggestion
@@ -112,2 +112,3 @@ - fireEvent.click(link); - + fireEvent.click(link); + expect(preventDefaultSpy).toHaveBeenCalled(); +
-
Review Details
-
Files reviewed - 6 · Commit Range:
da4fbd7..e5a2129- superset-frontend/src/components/Accessibility/SkipLink.test.tsx
- superset-frontend/src/components/Accessibility/SkipLink.tsx
- superset-frontend/src/components/Accessibility/StatusAnnouncer.test.tsx
- superset-frontend/src/components/Accessibility/StatusAnnouncer.tsx
- superset-frontend/src/components/Accessibility/index.tsx
- superset-frontend/src/dashboard/components/SliceHeader/index.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
|
|
||
| const SkipLink: FC<SkipLinkProps> = ({ | ||
| targetId = 'main-content', | ||
| children = 'Skip to main content', |
Contributor
There was a problem hiding this comment.
Missing translation for user-facing text
The default children prop 'Skip to main content' is hardcoded and should be wrapped with the translation function t() to support internationalization, as per the project's i18n standards.
Code suggestion
Check the AI-generated fix before applying
import { FC, MouseEvent } from 'react';
+import { t } from '@apache-superset/core';
import { styled } from '@apache-superset/core/ui';
@@ -63,1 +64,1 @@
- children = 'Skip to main content',
+ children = t('Skip to main content'),
Code Review Run #e1a659
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
- Add role="img" and aria-label to Chart container with chart name and viz type - Add visually-hidden screen-reader text to AlertStatusIcon with status label - Add visually-hidden screen-reader text to TaskStatusIcon with status label - Mark decorative icon SVGs as aria-hidden="true" - Add unit tests for Chart container accessibility attributes - Add unit tests for AlertStatusIcon screen-reader text across all states Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…(WCAG 1.1.1) - Add aria-label to FullscreenExitOutlined button in SliceHeaderControls - Add aria-label to Undo/Redo buttons in Dashboard Header (matching tooltip text) - Mark decorative icons as aria-hidden in SliceHeaderControls download menu - Mark decorative icons as aria-hidden in Header (Undo, Redo, Save icons) - Update BaseIconComponent to respect aria-hidden prop (role=presentation, no aria-label) - Mark decorative icons in Explore header, SQL Lab buttons (Save, Share, Download, Copy) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add aria-pressed to auto-refresh pause/resume toggle button - Add aria-pressed to edit dashboard mode toggle button - Add aria-label to DrillBy display mode radio group - Add aria-expanded and aria-label to filter bar collapse/expand buttons - Add tabIndex to collapsed filter bar for keyboard accessibility - Add tests for aria-pressed on edit and auto-refresh toggle buttons - Add tests for aria-expanded on filter bar collapse/expand controls Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…essages (WCAG 1.4.1) - Replace generic icons in AlertStatusIcon with distinct shapes per status: Success=CheckCircleOutlined, Error=CloseCircleOutlined, Working=LoadingOutlined, Noop=ClockCircleOutlined, Grace=ExclamationCircleOutlined - Add "Error:" text prefix to ChartErrorMessage and ImportModal ErrorAlert - Add aria-label with scheme name and role="option" to ColorSchemeLabel swatches - Mark individual color swatches as aria-hidden for cleaner screen reader output - Add tests verifying distinct icon rendering per status, icon presence in ErrorAlert, and aria-label on ColorSchemeLabel
- Add aria-atomic="true" to base Alert component for complete message announcement - Add aria-atomic="true" to BasicErrorAlert custom div container - Add WCAG 3.3.1 accessibility tests to Alert, BasicErrorAlert, and ErrorAlert - Verify role="alert", aria-atomic, aria-live attributes across error components - Test assertive aria-live for errors, polite for warnings/info/success Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… 3.3.1) - ModalFormField: auto-inject aria-invalid/aria-describedby on child inputs when error prop is set - LabeledErrorBoundInput: add aria-invalid and aria-describedby linking to error messages - SqlAlchemyForm: add on-blur validation with aria-invalid for Display Name and URI fields - SSHTunnelForm: add on-blur validation with aria-invalid for SSH Host, Port, Username fields - AlertReportModal: add field-level error messages for name and owners via ModalFormField - NumberInput: extend interface to accept aria-invalid and aria-describedby props - TextControl: add aria-invalid/aria-describedby and visible error span when validationErrors present - NumberControl: add aria-invalid/aria-describedby and visible error span when validationErrors present - ControlHeader: add sr-only error span with ID for aria-describedby linkage - RoleFormItems: add aria-required to role name input Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…erer - Replace CanvasRenderer with SVGRenderer in Echart.tsx for all 13+ chart types - All chart text (labels, axes, legends) now renders as SVG DOM elements, not canvas bitmaps - Text is selectable, scalable, and accessible to assistive technology - Add role="img", aria-label, and <title> to chart-card-fallback.svg - Tooltips remain unaffected (already HTML DOM elements via appendToBody) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…xt resize - Replace hardcoded 12px font-sizes with inherit/theme tokens in error messages - Convert fixed height:24px to min-height:24px in SliceHeader controls - Replace px-based line-heights with unitless values (1, 1.2, 1.5) - Remove fixed max-width on brand text, add flex-wrap for nav wrapping at zoom - Replace fontSize:22 with theme.fontSizeXL in fullscreen icon - Replace font-size:14px with theme.fontSize in ScheduleQueryButton - Replace fontSize:'12px' with theme.fontSizeSM in SparklineCell tooltip - Replace font-size:12px with theme.fontSizeSM in CustomizationsBadge icon Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add media queries to dashboard components for narrow viewports - Stack filter sidebar above content at mobile breakpoints - Make modal dialogs full-width on small screens - Prevent horizontal overflow in navigation header Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace colorSplit (~1.04:1) with colorTextTertiary (~3.54:1) for ECharts axis/grid lines - Add global CSS overrides for Ant Design input borders (colorTextTertiary, 3.54:1) - Strengthen focus indicators: colorPrimaryBorderHover → colorPrimary (~4.68:1) - Fix controlOutline in TimeFilterPlugin focus shadow (was low-alpha, now colorPrimary) - Replace colorPrimaryBorder with colorPrimary in filter focus highlight styles - Override Ant Design divider border color to colorTextTertiary - Replace colorSplit in RangeFilterPlugin divider with colorTextTertiary Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add responsive wrapping to SubMenu at 480px breakpoint - Add full-width filter controls at narrow viewports (GroupByFilterCard) - Add responsive max-width override for FilterDivider h3 at 480px Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Code Review Agent Run #cf51e6Actionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
This was referenced Apr 9, 2026
Contributor
Author
Member
|
Thanks for the huge amount of work here. Given that this has been split into 16 per-WCAG-criterion PRs (#39230–#39245) and the branch is now conflicting with master, I'd suggest closing this one and letting the individual PRs carry the review. Happy to continue reviewing the individual PRs once we confirm this can be closed. |
Member
|
@Aitema-gmbh thanks for picking this back up after #37211. Before I can take another deeper pass, a few things need attention:
Also: at 154 files / +6.2k lines this PR is genuinely too large to review well in one go. Could you split it into focused PRs — at minimum separating (1) the new Let me know once those are addressed and I'll take another look. |
…guide (QA-04) - WCAG Compliance Matrix mapping all 15 criteria to files, commits, and tests - Accessibility Release Notes documenting all changes by sprint - Developer Guide with 9 reusable accessibility patterns and code examples Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…UG-07) - BUG-01 (2.4.6): Add sr-only h1 headings to DashboardList, ChartList, SqlLab - BUG-02 (3.3.3): Login error with correction suggestion, aria-describedby, aria-invalid - BUG-03 (1.4.11/2.4.7): Override Ant Design focus ring with !important for 3:1 contrast - BUG-04 (1.4.3): Change link color from #2893B3 (3.55:1) to #0d7090 (5.62:1) - BUG-05 (2.4.1): Integrate SkipLink into App.tsx with main-content target - BUG-06 (4.1.3): Integrate StatusAnnouncerProvider into App.tsx - BUG-07 (4.1.2): Add aria-label to icon-only buttons (More, Import) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add html { font-size: 100% } in GlobalStyles.tsx to ensure base font
size respects browser preferences (WCAG 1.4.4)
- Replace React 18 useId() with useState() in Login/index.tsx and
Modal/ModalFormField.tsx for React 17 compatibility
- Fixes login page crash: "TypeError: useId is not a function"
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The error message now avoids assuming credential failure, while still providing a correction suggestion as required by WCAG 3.3.3. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1.4.4: Convert 12 font-size px declarations to rem across SubMenu, UserInfo, Modal, DatabaseModal, static error pages, CountryMap 1.4.11: Override Ant Design checkbox/radio/switch borders to colorTextTertiary (~3.54:1), enforce ECharts line-width >= 2px, fix table borders and progress bar visibility 1.4.13: Add ESC-dismiss and controlled state to Tooltip and Popover wrappers, make ECharts tooltips hoverable (enterable: true) with ESC keydown handler on chart container Also fix: StatusAnnouncer WCAG comment (4.1.2 → 4.1.3), remove dead link color rule in GlobalStyles Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add autoComplete="off" to OAuth2ClientField secret input - Replace non-standard dynamic autoComplete values with "off" in ImportModal (password, ssh_tunnel_password, ssh_tunnel_private_key, ssh_tunnel_private_key_password) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Darken 4 categorical palette colors that had insufficient contrast against white backgrounds (WCAG 1.4.11 non-text contrast): - #5AC189 -> #4DA575 (green, 2.23:1 -> 3.02:1) - #FF7F44 -> #E8733D (orange, 2.51:1 -> 3.02:1) - #FCC700 -> #B58F00 (yellow, 1.58:1 -> 3.05:1) - #3CCCCB -> #30A3A2 (cyan, 1.96:1 -> 3.05:1) Also update sequential/diverging palettes (superset_seq_2, superset_div_2). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- SqlEditorLeftBar: add @media (max-width: 768px) to override min-width: 500px - SaveDatasetModal: add @media (max-width: 480px) for form field max-width - Vertical FilterBar: add @media (max-width: 768px) display: none as safety net All fixed-width components now reflow correctly at 320px CSS width. Data tables exempt per WCAG (two-dimensional content). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Datasets was the only main page missing a screen-reader-only h1. Matches the pattern used on Dashboards, Charts and SQL Lab. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
One link on the Home page inherited colorPrimary (3.55:1) via
higher-specificity styled-component rules, bypassing the global
a { color: #0d7090 } rule (5.62:1). Add !important to ensure
all links meet the 4.5:1 contrast threshold.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous !important override applied to all <a> elements, which could break white-on-dark button links. Narrow selector to exclude ant-btn and role="button" elements. Verified on live deployment (v13) that primary buttons, filled buttons, and link-style buttons retain correct colors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds accessible names to 30+ icon-only interactive elements that were previously announced as just "button" by screen readers: - SubMenu: pass aria-label prop to import buttons (Dashboard, Chart, SavedQuery, Theme) - ListView: aria-label on grid/list view toggle buttons - Search filter: labeled clear icon via allowClear config - FaveStar: dynamic "Add to/Remove from favorites" label - Action buttons (Edit, Export, Delete, Duplicate, Cancel, Sync) on all list pages: Dashboard, Chart, Dataset, Database, Tags, RLS, Task - Explore: collapse/open datasource panel buttons - Dashboard: IconButton aria-label fallback, Tabs close icon Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1.4.5: Explicitly set renderer:'svg' in ECharts init() to guarantee SVG output. The full 'echarts' import in ZoomConfigsChart registered CanvasRenderer as a side effect, overriding the modular SVGRenderer. Migrated ZoomConfigsChart and zoomUtil to echarts/core modular imports. 1.4.1: Add distinct dash patterns and symbols per line series so information is not conveyed by color alone. Applies to both Timeseries and MixedTimeseries chart types. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MR-01: Add aria-labels to 3 icon-only buttons in dashboard edit mode
(DeleteComponentButton, Column settings, Row settings)
MR-02: Fix secondary button contrast — use colorPrimaryText instead of
colorPrimaryTextHover for WCAG 1.4.11 compliance (>= 3:1)
MR-03: Remove overflow-x:hidden from header at narrow viewports to allow
proper menu item reflow at 400% zoom (WCAG 1.4.10)
MR-04: Remove tabIndex={-1} from brand link to restore keyboard navigation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
colorPrimaryText has sufficient contrast in light mode but fails in dark mode (~2.7:1 on colorPrimaryBg). Use colorText instead, which provides >= 4:1 contrast in both light and dark mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ant Design menubar sets tabIndex=-1 on all items except the first, requiring arrow-key navigation. This useEffect override ensures every top-level menu item (including submenus like "SQL Lab") gets tabIndex=0, making them focusable via Tab key as users expect. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Aitema-gmbh
force-pushed
the
fix/a11y-improvements-v3
branch
from
a7c2b25 to
f54a896
Compare
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
This PR introduces accessibility improvements to Apache Superset for WCAG 2.1 Level A compliance. It supersedes #37211 with a clean rebase on current
masterand addresses all review feedback from the previous PR.New Accessibility Components
SkipLink: Skip-to-content navigation for keyboard users (WCAG 2.4.1 Bypass Blocks)tabindexmanagement — sets-1only during focus, removes on blur to preserve natural tab orderStatusAnnouncer: ARIA live region for screen reader announcements (WCAG 4.1.2 Name, Role, Value)useStatusAnnouncerhookpoliteandassertivepoliteness levelsEnhanced Semantic Structure
SliceHeader: Added semantic<h2>headings for chart titles (WCAG 1.3.1 Info and Relationships)Changes from previous PR #37211
master(fixes stale import paths)@apache-superset/core/uiand@superset-ui/core/componentsimport structuretabindexbug — now temporary instead of permanent (addresses Bito, CodeAnt, Copilot feedback)ActionButtons— upstream already has correct keyboard/filter logicWCAG Criteria Addressed
<h2>headings for chart titlesTesting Instructions
<h2>chart titles are properly announced with heading levelChecklist
CodeAnt-AI Description
Improve accessibility across charts, forms, navigation, and error handling
What Changed
Impact
✅ Faster keyboard navigation✅ Clearer screen reader announcements✅ Fewer form-entry and login errors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.